fix(releases): Prevent row-lock contention on last_seen bump#115443
Merged
Conversation
…aseProjectEnvironment and ReleaseEnvironment High-volume releases cause concurrent workers to pile up on the same row's UPDATE for last_seen, hitting statement_timeout (SENTRY-5HQZ). This adds a cache-based distributed lock so only one worker per row per 60s attempts the DB update, and catches OperationalError so a failed bump doesn't abort the rest of the event save pipeline. Fixes SENTRY-5HQZ
github-actions
Bot
added
the
Scope: Backend
wedamija
approved these changes
May 12, 2026
| if cache.add(bump_key, "1", timeout=60): | ||
| try: | ||
| cls.objects.filter( | ||
| id=instance.id, last_seen__lt=datetime - timedelta(seconds=60) |
Member
There was a problem hiding this comment.
This could probably be updated to last_seen__lt=datetime now, since the lock is doing the work for us
| if cache.add(bump_key, "1", timeout=60): | ||
| try: | ||
| cls.objects.filter( | ||
| id=instance.id, last_seen__lt=datetime - timedelta(seconds=60) |
The cache lock handles the 60s throttle now, so the SQL filter only needs to prevent setting last_seen backwards.
markstory
approved these changes
May 13, 2026
Comment on lines
+69
to
+81
| if cache.add(bump_key, "1", timeout=60): | ||
| try: | ||
| cls.objects.filter(id=instance.id, last_seen__lt=datetime).update( | ||
| last_seen=datetime | ||
| ) | ||
| except OperationalError: | ||
| metric_tags["bumped"] = "error" | ||
| return instance | ||
| instance.last_seen = datetime | ||
| cache.set(cache_key, instance, 3600) | ||
| metric_tags["bumped"] = "true" | ||
| else: | ||
| metric_tags["bumped"] = "skipped" |
Member
There was a problem hiding this comment.
This is a use-case that our buffers system can handle. We currently use buffers to handle updating last_seen times on Group and Release models. It could be used here as well. However, what you have is operationally lighter and if we don't need to capture the tail of each update set it will work well.
Member
Author
There was a problem hiding this comment.
oh I didnt even consider that, but yea I think pragmatically maybe its better to just keep this simple way here, and this is much less critical to get absolutely correct
Comment on lines
+91
to
+103
| if cache.add(bump_key, "1", timeout=60): | ||
| try: | ||
| cls.objects.filter(id=instance.id, last_seen__lt=datetime).update( | ||
| last_seen=datetime | ||
| ) | ||
| except OperationalError: | ||
| metrics_tags["bumped"] = "error" | ||
| return instance | ||
| instance.last_seen = datetime | ||
| cache.set(cache_key, instance, 3600) | ||
| metrics_tags["bumped"] = "true" | ||
| else: | ||
| metrics_tags["bumped"] = "skipped" |
Member
There was a problem hiding this comment.
With two of these (and potentially more) should we have a context manager to encapsulate this behavior?
with periodic_update(key=bump_key, timeout=60, metrics_tags=metrics_tags):
cls.objects.filter(id=instance.id, last_seen__lt=datetime).update(last_seen=datetime)
Member
Author
There was a problem hiding this comment.
yea I wondered about the duplication, that a nce wrapper though, ill try and see if I can reduce it to something clean since it has an early return there and a cache update
The throttled last_seen bump pattern was duplicated across ReleaseEnvironment and ReleaseProjectEnvironment. Extract it into sentry/utils/last_seen.py so both models share one implementation.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Model doesn't have a last_seen attribute as far as mypy knows. Using Any avoids the attr-defined errors without needing a Protocol.
…ted rows Use a HasLastSeen Protocol to type the instance parameter instead of Any. Keep model_class as Any since typing the Django manager chain is impractical. Also restore the bumped=false metric tag when a row is newly created, which was dropped during the extraction.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UPDATE last_seenon the sameReleaseProjectEnvironment/ReleaseEnvironmentrow, triggering PostgreSQLstatement_timeoutcancellations (SENTRY-5HQZ — 6105 occurrences).OperationalErroraborts the rest of the event save pipeline (nodestore persistence, release counts, group release records), causing silent data loss.cache.add-based distributed lock so only one worker per row per 60s attempts the DB update, eliminating the thundering herd. Also catchesOperationalErroras a safety net so a failed best-effortlast_seenbump never blocks event processing.ReleaseProjectEnvironmentandReleaseEnvironmentwhich had identical vulnerable patterns.Fixes SENTRY-5HQZ
Test plan
test_bump_skipped_when_cache_lock_held— verifies second worker skips the DB update when the cache lock is heldtest_bump_survives_operational_error— verifiesOperationalErroris caught and instance is returned